fix(daemon): handle Codex stream reconnect events#2274
Conversation
lefarcen
added
size/S
Siri-Ray
left a comment
Siri-Ray
left a comment
There was a problem hiding this comment.
@VIVAAN-DHAWAN I reviewed the Codex JSON stream reconnect handling change and the added daemon parser coverage. The new helper keeps the existing timeout reconnect behavior, treats the reported stream disconnected before completion reconnect frame as recoverable status, and preserves fatal handling for non-reconnect errors that merely mention reconnect text. I also ran pnpm --dir apps/daemon exec vitest run -c vitest.config.ts tests/json-event-stream.test.ts, which passed with 23 tests. Thanks for the focused fix and the regression coverage.
lefarcen
left a comment
lefarcen
left a comment
There was a problem hiding this comment.
Hey @VIVAAN-DHAWAN — the reconnect failure mode and targeted parser coverage are clearly described here. One small PR-body cleanup before this moves through maintainer review: could you add the ## Surface area checklist and a short ## Bug fix verification note for the red→green path, e.g. that the reported Codex reconnect frame now stays recoverable while the non-reconnect error case remains fatal? Your existing Summary and Verification sections already cover the why/what/validation, so no need to rename those.
|
I will check on it |
3 participants
Summary
Reconnecting...JSON error frames forstream disconnected before completionas recoverable status events instead of fatal adapter errors.Fixes #2268.
Verification
PATH="$HOME/.nvm/versions/node/v24.14.1/bin:$PATH" pnpm --dir apps/daemon exec vitest run -c vitest.config.ts tests/json-event-stream.test.tsPATH="$HOME/.nvm/versions/node/v24.14.1/bin:$PATH" pnpm --dir apps/daemon exec vitest run -c vitest.config.ts tests/json-event-stream.test.ts -t "stream disconnect|non-reconnect"PATH="$HOME/.nvm/versions/node/v24.14.1/bin:$PATH" pnpm --dir apps/daemon run typecheckgit diff --checkNotes
I also tried the full daemon suite. It initially failed because
better-sqlite3had been installed under Node 25 while the repo requires Node 24. After rebuildingbetter-sqlite3under Node 24, the full suite command stopped producing progress and had to be stopped manually, so I am not listing it as passing.